Skip to content

refactor: make clarity-cli to use clap #6720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
simone-stacks wants to merge 19 commits into
base: develop
Choose a base branch
from
Open

refactor: make clarity-cli to use clap #6720

simone-stacks wants to merge 19 commits into from
+2,185 −1,335

Conversation

@simone-stacks
Copy link

@simone-stacks simone-stacks commented Nov 27, 2025

Description

Refactors the clarity-cli command-line interface to use clap for argument parsing, replacing the previous manual argument handling.

Applicable issues

Additional info (benefits, drawbacks, caveats)

Benefits:

  • Auto-generated --help output for all commands and subcommands
  • Better error messages for invalid arguments
  • Cleaner, more maintainable argument parsing code

Checklist

  • Test coverage for new or modified code paths
  • Changelog is updated
  • Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
  • New clarity functions have corresponding PR in clarity-benchmarking repo

@codecov
Copy link

codecov bot commented Nov 27, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 8.02920% with 252 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.60%. Comparing base (e3afd65) to head (ab3ef82).

Files with missing lines Patch % Lines
contrib/clarity-cli/src/main.rs 8.02% 252 Missing ⚠️

❌ Your project check has failed because the head coverage (68.60%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #6720      +/-   ##
===========================================
- Coverage    73.03%   68.60%   -4.44%     
===========================================
  Files          580      580              
  Lines       360229   360393     +164     
===========================================
- Hits        263086   247235   -15851     
- Misses       97143   113158   +16015
Files with missing lines Coverage Δ
contrib/clarity-cli/src/lib.rs 75.91% <ø> (+1.96%) ⬆️
contrib/stacks-inspect/src/main.rs 0.00% <ø> (ø)
contrib/clarity-cli/src/main.rs 8.33% <8.02%> (+8.33%) ⬆️

... and 394 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e3afd65...ab3ef82. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@simone-stacks
Copy link
Author

simone-stacks commented Nov 28, 2025

stacks-core/contrib/clarity-cli/src/lib.rs

Lines 438 to 493 in 90033c3

/// Execute program in a transient environment. To be used only by CLI tools
/// for program evaluation, not by consensus critical code.
pub fn vm_execute_in_epoch(
program: &str,
clarity_version: ClarityVersion,
epoch: StacksEpochId,
) -> Result<Option<Value>, VmExecutionError> {
let contract_id = QualifiedContractIdentifier::transient();
let mut contract_context = ContractContext::new(contract_id.clone(), clarity_version);
let mut marf = MemoryBackingStore::new();
let conn = marf.as_clarity_db();
let mut global_context = GlobalContext::new(
false,
default_chain_id(false),
conn,
LimitedCostTracker::new_free(),
epoch,
);
global_context.execute(|g| {
let parsed =
ast::build_ast(&contract_id, program, &mut (), clarity_version, epoch)?.expressions;
eval_all(&parsed, &mut contract_context, g, None)
})
}
/// Execute program in a transient environment in the latest epoch.
/// To be used only by CLI tools for program evaluation, not by consensus
/// critical code.
/// TODO: Unused internally in the cli, but used in other functions - Shall we use vm_execute_in_epoch with StacksEpochId::latest() instead, so we can remove this function?
pub fn vm_execute(
program: &str,
clarity_version: ClarityVersion,
) -> Result<Option<Value>, VmExecutionError> {
let contract_id = QualifiedContractIdentifier::transient();
let mut contract_context = ContractContext::new(contract_id.clone(), clarity_version);
let mut marf = MemoryBackingStore::new();
let conn = marf.as_clarity_db();
let mut global_context = GlobalContext::new(
false,
default_chain_id(false),
conn,
LimitedCostTracker::new_free(),
StacksEpochId::latest(),
);
global_context.execute(|g| {
let parsed = ast::build_ast(
&contract_id,
program,
&mut (),
clarity_version,
StacksEpochId::latest(),
)?
.expressions;
eval_all(&parsed, &mut contract_context, g, None)
})
}

I noticed that vm_execute and vm_execute_in_epoch are basically the same function - with the only difference that vm_execute just hardcodes StacksEpochId::latest().

I think we could either remove vm_execute and update all callers to pass the epoch explicitly, or turn it into a simple wrapper that calls vm_execute_in_epoch with StacksEpochId::latest() parameter.

What do you think? If it makese sense, I'd tackle this in a separate PR.

@simone-stacks simone-stacks marked this pull request as ready for review November 28, 2025 16:30
@simone-stacks simone-stacks requested a review from a team December 9, 2025 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@simone-stacks simone-stacks

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@simone-stacks